Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Native Flow Response reply type definition #282

Merged
merged 6 commits into from
Dec 29, 2023

Conversation

pjvds
Copy link
Contributor

@pjvds pjvds commented Dec 27, 2023

This PR adds the type definition for an interactive nfp_reply message. This message is received when sharing an address, or completing an flow.

@Secreto31126
Copy link
Owner

Thanks for the PR! I didn't know this webhook notification existed. There are two nit details which will trigger a CI error, and one from the official documentation:

Here it says the "interactive" object will also contain an "action" property, and the "nfm_reply" might contain a "name" and "body" strings.

If you don't mind, could you fix and add those extra types? If not, don't worry, I can keep working from this.

Thanks!

@Secreto31126
Copy link
Owner

Prefer using unknown over any, since it's type safer and doesn't make ESLint cry :)

Also, run npm run prettier to fix the nit issues.

@pjvds
Copy link
Contributor Author

pjvds commented Dec 28, 2023

Got ya, will fix the issues and run linting locally before proposing the final changes. Thanks!

@Secreto31126
Copy link
Owner

I added 2 more commits to the PR, super minor nit picks and replaced "address_response" with "address_message" based on the documentation.

Let me know if there's something else you would like to add, and if not I can merge it and release it tomorrow.

Thanks for contributing!

@pjvds
Copy link
Contributor Author

pjvds commented Dec 29, 2023

Awesome, this all looks good now. I do have some other changes, but they are not related to this one. They have to deal with post() being non-blocking and serverless environments don't like those scenario's.

Anyhow... thanks for the great work!

@Secreto31126 Secreto31126 merged commit f2b35d4 into Secreto31126:main Dec 29, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants